Skip to content

Conversation

@erickcestari
Copy link
Contributor

This PR exposes internal onion decoding types and functions behind the fuzzing cfg flag to enable differential fuzzing against other Lightning implementations.

Changes

  • Move Hop, OnionDecodeErr, NextPacketBytes, and decode_next_payment_hop into fuzzy_onion_utils module (exposed only with #[cfg(fuzzing)])
  • Make payment_hash parameter optional in decode_next_payment_hop to support fuzzing scenarios where onion decoding needs to be tested independently

Closes #4247

Preparatory commit that exposes onion decoding types and functions
to fuzzing targets for differential fuzzing against other Lightning
implementations.

Affected types: Hop, OnionDecodeErr, NextPacketBytes
Affected function: decode_next_payment_hop
Allow passing None for payment_hash to support differential fuzzing
scenarios where onion decoding needs to be tested independently of
payment hash.
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Dec 11, 2025

I've assigned @tankyleo as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@TheBlueMatt
Copy link
Collaborator

Can you not fuzz the already-public https://docs.rs/lightning/latest/lightning/ln/onion_payment/fn.peel_payment_onion.html ? That would also include additional HTLC validation logic that would be good to fuzz vs just the low-level decoding logic.

@erickcestari
Copy link
Contributor Author

Can you not fuzz the already-public https://docs.rs/lightning/latest/lightning/ln/onion_payment/fn.peel_payment_onion.html ? That would also include additional HTLC validation logic that would be good to fuzz vs just the low-level decoding logic.

The current target I'm working on is designed to decode the onion only. There is a custom mutator that helps the fuzzer build valid onion packets that the Lightning implementations will unwrap.

I think it would be worthwhile to have a higher-level target for fully validating an update_add_htlc message, but it would be a different target.

If you want to see:
bitcoinfuzz/bitcoinfuzz#330

@codecov
Copy link

codecov bot commented Dec 11, 2025

Codecov Report

❌ Patch coverage is 62.50000% with 81 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.36%. Comparing base (fd85279) to head (57c9827).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/onion_utils.rs 62.14% 80 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4276      +/-   ##
==========================================
- Coverage   89.37%   89.36%   -0.02%     
==========================================
  Files         180      180              
  Lines      139395   139399       +4     
  Branches   139395   139399       +4     
==========================================
- Hits       124591   124576      -15     
- Misses      12216    12235      +19     
  Partials     2588     2588              
Flag Coverage Δ
fuzzing 35.18% <19.90%> (-0.01%) ⬇️
tests 88.70% <62.50%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TheBlueMatt
Copy link
Collaborator

Right, but you can fuzz something higher-level and generally get the same results, just with more checks so that its more realistic in terms of how LDK handles it (if theres a crash in higher-level logic we want to know!). You might have to allow a few more cases where our higher-level logic rejects things, but that should be doable.

@erickcestari
Copy link
Contributor Author

Right, but you can fuzz something higher-level and generally get the same results, just with more checks so that its more realistic in terms of how LDK handles it (if theres a crash in higher-level logic we want to know!). You might have to allow a few more cases where our higher-level logic rejects things, but that should be doable.

Differential fuzzing requires comparing equivalent operations across implementations with matching inputs and outputs. The current target is specifically designed to test onion decoding in isolation, using a custom mutator that builds valid onion packets for each implementation to unwrap.

If I fuzz peel_payment_onion in LDK, the additional HTLC validation logic would cause output differences unrelated to onion decoding correctness, making it harder to identify real discrepancies.

A higher-level target that validates full update_add_htlc messages would be valuable, but it would be a separate fuzz target with different goals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Make onion_utils Public Behind a Feature Flag

3 participants